Skip to content

parser: support new syntax for extended statistics - #913

Merged
eurekaka merged 2 commits into
pingcap:masterfrom
eurekaka:ext_stats
Jul 30, 2020
Merged

eurekaka merged 2 commits into
pingcap:masterfrom
eurekaka:ext_stats

Conversation

@eurekaka

@eurekaka eurekaka commented Jul 3, 2020 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Parser changes of pingcap/tidb#18330

What is changed and how it works?

Add new SQL syntax CREATE STATISTICS / DROP STATISTICS / ADMIN RELOAD STATISTICS support.

Check List

Tests

  • Unit test

Code changes

  • Has exported variable/fields change

Side effects

N/A

Related changes

  • Need to update the documentation

@eurekaka eurekaka added type/enhancement New feature or request type/new-feature labels Jul 3, 2020
@codecov

codecov Bot commented Jul 3, 2020 •

Copy link
Copy Markdown

Codecov Report

Merging #913 into master will increase coverage by 0.10%.
The diff coverage is 76.00%.

@@            Coverage Diff             @@
##           master     #913      +/-   ##
==========================================
+ Coverage   78.37%   78.48%   +0.10%     
==========================================
  Files          40       40              
  Lines       14936    14984      +48     
==========================================
+ Hits        11706    11760      +54     
+ Misses       2538     2527      -11     
- Partials      692      697       +5     

@kennytm

kennytm commented Jul 3, 2020

Copy link
Copy Markdown
Contributor

Current syntax:

CREATE STATISTICS [IF NOT EXISTS] statistics_name
    [ '(' CARDINALITY ')' | '(' DEPENDENCY ')' | '(' CORRELATION ')' ] 
    ON table_name '(' column_name ',' column_name [',' ...] ')'

For comparison, PostgreSQL (and CockroachDB):

CREATE STATISTICS [IF NOT EXISTS] statistics_name
    [ '(' { NDISTINCT | DEPENDENCIES | MCV } [',' ...] ')' ]
    ON column_name ',' column_name [',' ...] 
    FROM table_name

Transact-SQL

CREATE STATISTICS statistics_name
    ON table_name '(' column_name ',' column_name [',' ...] ')'
    [ WHERE predicate ]
    [ WITH options ]

SAP HANA

CREATE STATISTICS statistics_name
    ON table_name [ '(' column_name ',' column_name [',' ...] ')' ] FOR { DEFAULT | EXTENDED } STORAGE
    [ TYPE statistics_type ]
    [ statistics_properties ',' statistics_properties [',' ...] ]
    [ [ NO ] INITIAL REFRESH ]

So the current syntax looks like a strange mix of PostgreSQL and Transact-SQL.

Question:

  • why not allow specifying multiple statistics?

    create statistics foo (cardinality, correlation) on tbl (c1, c2);

@eurekaka

eurekaka commented Jul 3, 2020

Copy link
Copy Markdown
Contributor Author

@kennytm the idea of extended statistics is mainly borrowed from PostgreSQL, but we plan to support different types of statistics, i.e, correlation. The definition of table and columns, i.e, ON t(a,b,c) is consistent with MySQL syntax style, e.g, create index idx on t(a,b,c).

@eurekaka eurekaka changed the title parser: support CREATE STATISTICS syntax parser: support new syntax for extended statistics Jul 22, 2020
@eurekaka
eurekaka force-pushed the ext_stats branch 2 times, most recently from f1e9207 to 9d24cf0 Compare July 27, 2020 11:21
@eurekaka
eurekaka requested review from qw4990 and winoros July 27, 2020 11:22
@eurekaka

Copy link
Copy Markdown
Contributor Author

CI failure should be fixed after pingcap/tidb#18787 is merged.

Comment thread parser_test.go

@qw4990 qw4990 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot

Copy link
Copy Markdown
Contributor

@qw4990,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: ddl(slack).

@qw4990 qw4990 added the status/LGT1 LGT1 label Jul 29, 2020

@winoros winoros left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot

Copy link
Copy Markdown
Contributor

@winoros,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: ddl(slack).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants